Skip to content

CI: Restrict trusted CircleCI PR authors to specific teams#34975

Merged
Sidnioulz merged 4 commits into
nextfrom
sidnioulz/restrict-trusted-pr-authors
Jun 8, 2026
Merged

CI: Restrict trusted CircleCI PR authors to specific teams#34975
Sidnioulz merged 4 commits into
nextfrom
sidnioulz/restrict-trusted-pr-authors

Conversation

@Sidnioulz

@Sidnioulz Sidnioulz commented May 29, 2026

Copy link
Copy Markdown
Contributor

What I did

Tightened security by only allowing Core team and trusted maintainers to write to the shared CircleCI cache.

Checklist for Contributors

Testing

ø

Manual testing

  • Add debug commit to this branch allowing to trigger the workflows when opening a PR that targets it
  • Create a dummy PR targeting it
  • Verify flag passed based on author identity
  • Clean up debug commit

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Chores
    • Improved CI reliability by strengthening pull-request author verification to check active membership across project teams and by passing the resulting trust flag into the pipeline trigger—reducing false positives/negatives and making automated pipeline runs more accurate and secure.

Copilot AI review requested due to automatic review settings May 29, 2026 14:20
@Sidnioulz Sidnioulz added build Internal-facing build tooling & test updates ci:normal Run our default set of CI jobs (choose this for most PRs). labels May 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens CI trust handling by checking whether PR authors belong to specific Storybook teams before allowing CircleCI to treat them as trusted for shared cache writes.

Changes:

  • Replaces broad OWNER/MEMBER author association trust with explicit GitHub team membership checks.
  • Adds a ghTrustedAuthor value intended to be passed through to CircleCI.
  • Temporarily expands push triggers for manual debug testing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated
Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated
Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed.

Review Change Stack

📝 Walkthrough

Walkthrough

The workflow replaces association-based trust checks with GitHub API team membership queries for storybookjs teams and passes the computed ghTrustedAuthor flag into the CircleCI pipeline parameters.

Changes

Team-based trusted author verification

Layer / File(s) Summary
Team-based trust verification logic and integration
.github/workflows/trigger-circle-ci-workflow.yml
Introduces GITHUB_API_TOKEN in the trusted-author step, replaces author_association logic with GitHub API queries (curl + jq) to check active membership in storybookjs teams (maintainers, core, developer-experience), updates job permissions to include members: read, and adds GH_TRUSTED_AUTHOR / --arg ghTrustedAuthor to the CircleCI PARAMETERS JSON construction.
sequenceDiagram
  participant PR_Author as PR Author
  participant GitHub_API as GitHub API
  participant GH_Workflow as GH Actions Workflow
  participant CircleCI as CircleCI Pipeline
  PR_Author->>GH_Workflow: open/modify PR triggers workflow
  GH_Workflow->>GitHub_API: GET /orgs/storybookjs/teams/{team}/memberships/{author} (auth with GITHUB_API_TOKEN)
  GitHub_API->>GH_Workflow: membership state (active|inactive|...)
  GH_Workflow->>GH_Workflow: compute ghTrustedAuthor if any state == "active"
  GH_Workflow->>CircleCI: POST trigger with PARAMETERS including ghTrustedAuthor
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • storybookjs/storybook#34853: Adds the ghTrustedAuthor pipeline parameter to CircleCI configuration and uses it to conditionally gate cache persistence based on author trust status.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/trigger-circle-ci-workflow.yml:
- Around line 152-157: The jq-generated PARAMETERS JSON is missing the
ghTrustedAuthor field despite the --arg ghTrustedAuthor being passed; update the
PARAMETERS construction (the jq invocation that sets PARAMETERS) to include
"ghTrustedAuthor: $ghTrustedAuthor" in the object so the CircleCI payload
actually contains that key (this ensures scripts/ci/utils/parameters.ts receives
the value and trusted-author logic in scripts/ci/main.ts is honored).
- Around line 46-47: Remove the temporary debug push branch entry
"sidnioulz/restrict-trusted-pr-authors" from the workflow configuration so it no
longer widens the always-trusted push path; locate the string in the workflow
list (the line containing sidnioulz/restrict-trusted-pr-authors) and delete that
entry so only the intended trusted branches (e.g., main/next) remain.
- Around line 101-121: The workflow step that queries the org team membership
uses GITHUB_API_TOKEN (github.token) but the job currently denies token scopes;
update the job- or step-level permissions to grant orgs: read for github.token
so the GET /orgs/storybookjs/teams/$TEAM/memberships/$USER_LOGIN call can
succeed, and modify the loop that builds TRUSTED_TEAMS and calls curl (the curl
-H "Authorization: Bearer $GITHUB_API_TOKEN" ... | jq -r '.state' 2>/dev/null ||
true) to stop swallowing failures — remove the "|| true" and add explicit
non-200/parse handling (check curl/http status and jq output) so membership
lookup failures are treated as untrusted and logged instead of silently falling
through.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 773bd750-7b41-44ba-9eb3-264d1c764fab

📥 Commits

Reviewing files that changed from the base of the PR and between d6ce689 and aa83004.

📒 Files selected for processing (1)
  • .github/workflows/trigger-circle-ci-workflow.yml

Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated
Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated
Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated
@storybook-app-bot

storybook-app-bot Bot commented May 29, 2026

Copy link
Copy Markdown

Package Benchmarks

Commit: 331367e, ran on 5 June 2026 at 19:06:33 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 74 72 🎉 -2 🎉
Self size 20.43 MB 20.39 MB 🎉 -35 KB 🎉
Dependency size 36.65 MB 36.11 MB 🎉 -539 KB 🎉
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 205 203 🎉 -2 🎉
Self size 908 KB 908 KB 0 B
Dependency size 89.16 MB 88.58 MB 🎉 -573 KB 🎉
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 198 196 🎉 -2 🎉
Self size 32 KB 32 KB 🚨 +36 B 🚨
Dependency size 87.65 MB 87.07 MB 🎉 -573 KB 🎉
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 75 73 🎉 -2 🎉
Self size 1.08 MB 1.08 MB 0 B
Dependency size 57.08 MB 56.50 MB 🎉 -573 KB 🎉
Bundle Size Analyzer node node

@Sidnioulz Sidnioulz force-pushed the sidnioulz/restrict-trusted-pr-authors branch from aa83004 to 6189230 Compare May 29, 2026 14:36
@Sidnioulz Sidnioulz force-pushed the sidnioulz/restrict-trusted-pr-authors branch from 6189230 to 3bb369f Compare May 29, 2026 14:38
@Sidnioulz Sidnioulz self-assigned this May 29, 2026
@Sidnioulz Sidnioulz moved this to In Progress in Core Team Projects May 29, 2026
Co-authored-by: Sidnioulz <5108577+Sidnioulz@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/trigger-circle-ci-workflow.yml:
- Around line 80-81: The workflow currently sets an invalid permissions scope
"members: read" under the permissions block; remove that invalid scope (delete
"members: read") and either omit the permissions block entirely or replace it
with valid permissions your job needs (e.g., "contents: read", "actions: read",
etc.); if the job truly needs org/team membership data, do not rely on
GITHUB_TOKEN—instead require a PAT or GitHub App token with read:org provided
via a secret (reference the secret in the workflow) so team membership API calls
succeed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 46d7a999-fc7d-4d80-8763-234b65ef783c

📥 Commits

Reviewing files that changed from the base of the PR and between 3bb369f and 21a4ff6.

📒 Files selected for processing (1)
  • .github/workflows/trigger-circle-ci-workflow.yml

Comment thread .github/workflows/trigger-circle-ci-workflow.yml Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/trigger-circle-ci-workflow.yml (1)

113-127: 💤 Low value

Consider adding visibility for API call failures.

The || true on line 119 suppresses all errors, making the behavior fail-closed (secure: untrusted on failure). However, if STORYBOOKJS_ORG_MEMBERSHIP_TOKEN is misconfigured or the API is unavailable, all legitimate trusted users will silently lose cache-write access with no indication of why.

Consider logging the HTTP status or distinguishing "not a member" from "API error":

♻️ Suggested improvement for observability
             for TEAM in $TRUSTED_TEAMS; do
-              STATE=$(curl -fsSL \
+              RESPONSE=$(curl -sS -w "\n%{http_code}" \
                 -H "Authorization: Bearer $GITHUB_API_TOKEN" \
                 -H "Accept: application/vnd.github+json" \
                 -H "X-GitHub-Api-Version: 2022-11-28" \
-                "https://api.github.com/orgs/storybookjs/teams/$TEAM/memberships/$USER_LOGIN" \
-                | jq -r '.state' 2>/dev/null || true)
+                "https://api.github.com/orgs/storybookjs/teams/$TEAM/memberships/$USER_LOGIN" 2>&1) || true
+              HTTP_CODE=$(echo "$RESPONSE" | tail -n1)
+              BODY=$(echo "$RESPONSE" | sed '$d')
+              STATE=$(echo "$BODY" | jq -r '.state' 2>/dev/null || echo "")
+              
+              if [ "$HTTP_CODE" != "200" ] && [ "$HTTP_CODE" != "404" ]; then
+                echo "::warning::Team membership check for $TEAM returned HTTP $HTTP_CODE"
+              fi
 
               if [ "$STATE" = "active" ]; then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/trigger-circle-ci-workflow.yml around lines 113 - 127, The
curl call that determines TEAM membership swallows all errors via "|| true", so
failures (bad STORYBOOKJS_ORG_MEMBERSHIP_TOKEN or network/API issues) are
indistinguishable from "not a member"; update the logic around the curl/jq call
in the loop (the command that sets STATE and the variables TRUSTED_TEAMS, STATE,
IS_TRUSTED) to detect and log API errors: capture the HTTP status or stderr from
the curl invocation, treat non-2xx responses as an API error (do not silently
convert to empty STATE), and emit a clear echo or error line before deciding
IS_TRUSTED so failures are visible in workflow logs while still preserving the
existing active check that sets IS_TRUSTED when STATE == "active".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/trigger-circle-ci-workflow.yml:
- Around line 113-127: The curl call that determines TEAM membership swallows
all errors via "|| true", so failures (bad STORYBOOKJS_ORG_MEMBERSHIP_TOKEN or
network/API issues) are indistinguishable from "not a member"; update the logic
around the curl/jq call in the loop (the command that sets STATE and the
variables TRUSTED_TEAMS, STATE, IS_TRUSTED) to detect and log API errors:
capture the HTTP status or stderr from the curl invocation, treat non-2xx
responses as an API error (do not silently convert to empty STATE), and emit a
clear echo or error line before deciding IS_TRUSTED so failures are visible in
workflow logs while still preserving the existing active check that sets
IS_TRUSTED when STATE == "active".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2c629d4c-67ca-4cd2-9137-17b7f9b3d0ba

📥 Commits

Reviewing files that changed from the base of the PR and between 21a4ff6 and 331367e.

📒 Files selected for processing (1)
  • .github/workflows/trigger-circle-ci-workflow.yml

@Sidnioulz Sidnioulz merged commit 1ea9136 into next Jun 8, 2026
142 checks passed
@Sidnioulz Sidnioulz deleted the sidnioulz/restrict-trusted-pr-authors branch June 8, 2026 09:19
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Core Team Projects Jun 8, 2026
@github-actions github-actions Bot mentioned this pull request Jun 8, 2026
24 tasks
@github-actions github-actions Bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 9, 2026
@github-actions github-actions Bot mentioned this pull request Jun 10, 2026
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal Run our default set of CI jobs (choose this for most PRs). patch:done Patch/release PRs already cherry-picked to main/release branch

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants